feat(config): auto-detect supported config files in the working directory#901
feat(config): auto-detect supported config files in the working directory#901AysajanE wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Adds Reviewed by Cursor Bugbot for commit 4c8e448. Bugbot is set up for automated code reviews on this repo. Configure here. |
AugustinMauroy
left a comment
There was a problem hiding this comment.
adding blocker since feature isn't correctly covered.
thanks @AysajanE for you effort here
|
Thanks for the specific direction, @AugustinMauroy and @avivkeller. I have updated and rebased the branch to cover the full supported configuration-file set. When --config-file is omitted, discovery now checks this exact priority order:
The first existing file wins. An explicit configFile still bypasses discovery, JSON is not included because it is unsupported, and the no-file behavior remains unchanged. Coverage now includes:
Local verification on current upstream main b9750dc:
I also checked the still-open dynamic-configuration PR #905. It touches the same configuration area but implements dynamic default values, so there is no semantic overlap with this file-discovery correction at present. The updated head is 662687f. Could you please take another look when CI is ready? |
|
I feel like a dep like cosmiconfig does this for us? |
|
Didn't mean to close! Feel free to iterate based on my feedback |
|
Thanks — that makes sense. I have reworked the revision locally to use cosmiconfig for discovery instead of maintaining the file-probing logic ourselves. The explorer is configured for the current working directory and the agreed six doc-kit.config.* names, in priority order; an explicit --config-file still bypasses discovery, and the existing loader remains responsible for JavaScript and TypeScript module loading. Local verification is green: focused configuration tests 17/17, full suite 527/527, six-extension plus priority smoke check passed, lint has 0 errors (2 pre-existing unrelated warnings), and formatting passed. I have not updated the PR head yet. |
|
Can you push those changes? |
Signed-off-by: Aysajan Eziz <[email protected]>
|
Thanks — I’ve pushed the cosmiconfig revision in Configuration discovery is now scoped to the current working directory and uses the agreed priority order for Validation after the update:
Please take another look when convenient. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #901 +/- ##
==========================================
- Coverage 86.17% 85.17% -1.01%
==========================================
Files 188 192 +4
Lines 17146 17575 +429
Branches 1540 1565 +25
==========================================
+ Hits 14776 14969 +193
- Misses 2363 2599 +236
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| await cosmiconfig('doc-kit', { | ||
| searchPlaces: CONFIG_FILE_NAMES, | ||
| loaders: CONFIG_LOADERS, | ||
| searchStrategy: 'none', | ||
| }).search(process.cwd()) |
There was a problem hiding this comment.
Why is this so complex?
Can we not just:
const explorer = cosmiconfig('doc-kit');
const result = await explorer.search();
There was a problem hiding this comment.
You are right that search() already starts from process.cwd() and defaults to a non-traversing search, so the explicit working directory and searchStrategy are redundant.
The additional searchPlaces and loaders were preserving the six filenames requested in the earlier review: doc-kit.config.js, .cjs, .mjs, .ts, .cts, and .mts, with a defined priority. In particular, cosmiconfig defaults do not discover .cts or .mts, and using its defaults would also broaden discovery to package/rc/JSON/YAML conventions.
I can simplify this to cosmiconfig("doc-kit").search() if that earlier six-extension requirement is no longer desired. That would intentionally drop automatic .cts/.mts discovery and follow the standard cosmiconfig conventions instead, while explicit --config-file paths would remain unchanged. Could you confirm which behavior you prefer?
There was a problem hiding this comment.
I'd prefer that you simplified this, as I had requested, regardless of MTS/TS support.
I'd also prefer to I spoke to a human :-)
There was a problem hiding this comment.
Got it. I am reading every comment here as the real person Aysajan Eziz, as I am typing this manually.
Signed-off-by: Aysajan Eziz <[email protected]>
Problem
doc-kit generateonly loads configuration when--config-fileis passed explicitly —loadConfigFilereturns{}otherwise. As raised in #897, most config-driven tools also detect a conventional config file automatically, so a validdoc-kit.config.mjssitting in the project root is silently ignored today. That is an easy footgun for anyone who assumes the file is picked up.Change
createRunConfiguration, when no--config-fileis provided, auto-detect adoc-kit.config.mjsin the current working directory and load it through the existing loader. An explicit--config-filestill takes precedence, and when no file is present the configuration falls back to empty — unchanged behavior. No new dependency; a tight happy path, as the issue suggests.Checks (Node 22)
node --test --experimental-test-module-mocks src/utils/configuration/__tests__/index.test.mjs— 17 passing, including the new cases.Fixes #897
Disclosure: this change was prepared with AI assistance and reviewed by me before submitting.
I ran the check(s) above in a network-isolated container and published a signed, re-runnable
record of that run, verifiable via GitHub artifact attestation:
https://northset-oss.github.io/verification-pilot/. Contributor self-run, not a maintainer verification.